fix(privacy): make the phone chrome band an opaque bar, and give it a real edge - #2487
Conversation
… real edge The follow-up recorded in PR #2462 and again in #2473: the privacy route's sticky header read as a white band floating on a white page. Looking at it in a browser rather than reasoning about the class list turned up a second, worse defect that no screenshot of the top of the page could show. **The band was translucent at every width, so scrolled content ghosted through the title.** Scrolling to 600px put the amber obligation card behind the header, and its fill and "READ MORE" label read straight through "Data handling". The repo already has a rule for this: `.edge-glass-header` / `.universal-header` in globals.css set `background: var(--surface); backdrop-filter: none` under `max-width: 639px` — "Phone baseline: a full-width opaque header." This page had never followed it, carrying `--surface-glass`/95 plus `backdrop-blur-xl` at all widths. It now matches: opaque on phones, the shared 72%-surface glass from `sm`. **The edge was too light to anchor the band.** In the live v2 token layer `--background`, `--surface`, `--surface-chrome` and `--surface-raised` are all `#ffffff`, so no surface tone can separate this band from the page beneath it — the border and the elevation are the only signals available, and both were at their faintest: `--border` (#e6ebf2) over `--e1` (5% alpha, 2px blur). Measured in the browser, not inferred. The edge moves to `--border-strong` (#d3dbe5) and the lift to `--e2`, which is one step and still below the `--e3`/`--e4` reserved for hover chrome and modals. Two corrections to my own earlier reasoning, both caught by measuring: - I had assumed the page ground was the tinted `--background: #f1f4f8` from globals.css and that the band merely needed to match it. It is not: `.ckb-v2` is the layer in force and its `--background` is pure white, which is why no amount of re-tinting the band would have closed the seam. - A first attempt moved the band to the shared 72% glass at every width. That is the correct value for `sm`+ and made the phone case measurably worse, because 72% of white is more transparent than the 78% it replaced. The screenshot of the scrolled state is what caught it. Verification - `npm run verify:phone-chrome` — full gate green. lock-parity, runtime and the phone-chrome static/unit contracts pass; `changed-browser` ran the complete ui-smoke suite, `105 passed (3.5m)`; `focused-browser` `6 passed`. The gate selected focused ownership scope ("sufficient for this page-local scope") and did not escalate to full `verify:ui`. - `tests/ui-smoke.spec.ts` gains a browser-proven assertion, as the chrome contract requires for exactly this case: tailwind-merge keeps both the base and the `sm:` utility, so stylesheet order rather than the class list decides the winner, and jsdom can resolve neither. It asserts a fully opaque phone background with no alpha channel, `backdrop-filter: none`, that the H1 is genuinely hidden behind the band once scrolled, and that `sm` restores blur and a different background. - That assertion was mutation-tested by reintroducing the original bug verbatim (`--surface-glass`/95 + `backdrop-blur-xl` on phones). It failed on the alpha channel, reporting the real value `oklab(... / 0.778627)`, then restored. - Design-system contract passed, every ratchet at or under baseline. Privacy and chrome contract units: `88 passed` across 6 files. - Full unit suite `11770 passed | 1 skipped` across 931 files. Three pre-existing failures (`clinical-hazard-controls`, `privacy-readiness-contract`, `rag-plan-package-parity`) are unrelated and reproduce on a clean checkout. - Lint clean at `--max-warnings 0`; typecheck exit 0, unscoped; format committed with the work. RAG impact: no retrieval behaviour change - this diff touches no file under src/lib/rag/** and none of the protected ranking surfaces (clinical-search, retrieval-selection, released-search-order, ranking-config, answer-ranking, answer-verification), adds no comparator key, and leaves the golden fixture and eval harness untouched. Verified by listing the diff's two paths against that set. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YRGeyyiZNrXnFJjVVnykgp
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_a7a9e61e-39ae-403e-a1e8-aab16e98318a) |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Summary
The follow-up recorded in #2462 and again in #2473: the privacy route's sticky header read as a white band floating on a white page. Looking at it in a browser rather than reasoning about the class list turned up a second, worse defect that no screenshot of the top of the page could show.
.edge-glass-header/.universal-headeringlobals.csssetbackground: var(--surface); backdrop-filter: noneundermax-width: 639px— "Phone baseline: a full-width opaque header." This page had never followed it, carrying--surface-glass/95 plusbackdrop-blur-xlat every width. It now matches: opaque on phones, the shared 72%-surface glass fromsmup..ckb-v2token layer,--background,--surface,--surface-chromeand--surface-raisedare all#ffffff— so no surface tone can separate this band from the page beneath it, and the border and elevation are the only signals available. Both were at their faintest:--border(#e6ebf2) over--e1(5% alpha, 2px blur), measured in the browser rather than inferred. The edge moves to--border-strong(#d3dbe5) and the lift to--e2— one step, still below the--e3/--e4reserved for hover chrome and modals.Two corrections to my own earlier reasoning, both caught by measuring
--background: #f1f4f8fromglobals.css, and that the band merely needed to match it. It is not..ckb-v2is the layer in force and its--backgroundis pure white — which is why no amount of re-tinting the band would ever have closed the seam. I was reading the compatibility layer, not the live one.sm+ and made the phone case measurably worse, because 72% of white is more transparent than the 78% it replaced. The screenshot of the scrolled state is the only thing that caught it.Verification
npm run verify:phone-chrome— full gate green.lock-parity,runtimeand the phone-chrome static/unit contracts pass;changed-browserran the complete ui-smoke suite at105 passed (3.5m);focused-browser6 passed. The gate selected focused ownership scope ("Focused ownership and journey coverage is sufficient for this page-local or test-infrastructure scope") and did not escalate to fullverify:ui.tests/ui-smoke.spec.ts, as the chrome contract requires for exactly this case: tailwind-merge keeps both the base and thesm:utility, so stylesheet order rather than the class list decides the winner, and jsdom can resolve neither. It asserts a fully opaque phone background with no alpha channel,backdrop-filter: none, that the H1 is genuinely hidden behind the band once scrolled, and thatsmrestores blur and a different background.--surface-glass/95 +backdrop-blur-xlon phones). It failed on the alpha channel, reporting the real valueoklab(0.999994 … / 0.778627), and was then restored.npm run check:design-system-contract— passed, every ratchet at or under baseline. Adoption 55 components / 84 roots; design-sync 55 components / 7 guidelines.88 passedacrossprivacy-ui,privacy-page.dom,privacy,ui-overlay-css-contract,header-scroll-hide-contract,mobile-composer-reserve.npm run test—11770 passed | 1 skippedacross 931 files. Three pre-existing failures inclinical-hazard-controls,privacy-readiness-contractandrag-plan-package-parity, reproduced identically on a cleanorigin/maincheckout in this container (3 failed | 22 passed). I checked the privacy-named one specifically rather than assuming, since this diff touches the privacy page.npm run lintclean at--max-warnings 0;npm run typecheckexit 0, unscoped;npm run formatcommitted with the work.What is still not proven: this was verified on Chromium at 390px, 700px and 1280px. Physical Safari and an installed PWA remain unchecked, and the opaque-bar rule exists partly because of iOS toolbar behaviour — so the phone case deserves a real-device glance after it lands.
Risk and rollout
backdrop-filter, a border colour token and an elevation token on a single element, plus one test. The band's height, position and contents are untouched, so nothing that depends onstickyChromeHeightPxmoves.src/lib/rag/**and none of the protected ranking surfaces (clinical-search, retrieval-selection, released-search-order, ranking-config, answer-ranking, answer-verification), adds no comparator key, and leaves the golden fixture and eval harness untouched. Verified by listing the diff's two paths against that set.Clinical Governance Preflight
Clinical KB Database(sjrfecxgysukkwxsowpy)Notes against those items, in full rather than as ticks alone.
This touches the privacy page, which is a governance surface, so the boundary is worth stating precisely: no privacy copy, retention statement, processing description, obligation band or disclosure changed. Not a word of what the page tells a reader about how PsychSift handles data is different. The diff changes the background, blur, border colour and elevation of the sticky header bar, and adds a test.
The one governance-relevant improvement is legibility rather than content: the amber obligation card — the band carrying "Do not enter patient-identifiable information" — previously showed through the header while scrolling, which degrades the readability of the very warning the page exists to deliver. An opaque bar removes that interference. No threshold, wording or condition behind that warning was touched.
No Supabase target, migration, credential or document-access path is involved, and nothing added here runs server-side.
Failure behaviour is unchanged and remains conservative. The band is styled entirely in CSS with no new state, no new JavaScript and no runtime data: it renders identically whether or not anything resolves. The existing
chromeStickyguard — which drops sticky positioning when enlarged text makes the header taller than the viewport, so it cannot cover every accordion target — is untouched and still governs positioning.Notes
Not fixed here, and not this PR's to fix: Cursor Bugbot has now failed to run on three consecutive PRs against a Cursor usage/spend limit, so it has reviewed none of them. That is an account setting rather than a code problem, and only the owner can lift it.
Generated by Claude Code
Note
Low Risk
Scoped to one route’s sticky header CSS and a smoke test; no logic, data, or layout geometry changes.
Overview
The privacy page sticky header band no longer uses translucent glass at every breakpoint. On phones it is now a fully opaque
var(--surface)bar with no backdrop blur, aligned with the shared.edge-glass-header/.universal-headerphone rule inglobals.css, so scrolled content (including the amber obligation warning) does not show through “Data handling”. Fromsmup, the band keeps the shared 72% surface glass andbackdrop-blur-xl.The band’s visual anchor is strengthened: border token
--border-strongand elevation--e2replace the lighter border and--e1, since white-on-white surfaces cannot separate the chrome from the page alone.A ui-smoke test asserts computed phone vs wide styles (opaque background, no blur, H1 hidden when scrolled, blur restored at
sm) rather than relying on class names.Reviewed by Cursor Bugbot for commit 07df3f6. Configure here.